Skip to content

Refine test coverage #43

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 4, 2020
Merged

Refine test coverage #43

merged 8 commits into from
Jul 4, 2020

Conversation

pellared
Copy link
Member

@pellared pellared commented Jul 2, 2020

Addresses #41

@pellared pellared mentioned this pull request Jul 2, 2020
@masch
Copy link

masch commented Jul 2, 2020

It looks goog to me.

You have a great readme documentation, so What do you think about to add an entry on it about the feature of uploading the test coverage result as a github artifact?

@pellared
Copy link
Member Author

pellared commented Jul 3, 2020

Sure, I will try to enhance README.md. I am not good at writing, but I will do my best.

I tested the stuff on more project and it looks like -coverpkg=./... causes often trouble. The tests where panicing from unknown reason. I see two options:

  1. Use https://github.com/ory/go-acc. Read more.. I tested it and on some other project and worked fine. However the test execution has changed from ~20 sec to ~60 sec (sic!).
  2. Get rid of -coverpkg=./... and accept the fact that the toolchain recording code coverage for the package that is currently tested. Add FAQ section about it and how to get better results using go-acc.

@masch: What do you think?

@masch
Copy link

masch commented Jul 4, 2020

I think that the approach that you took is great! I think that you wrote a good explanation on why you decided it and also you gave a good alternative if someone want to use -coverpkg option.

If you let me one more comment please, I know it's none of my business, but I'm 🤓 on git commits history too.
In orde order to improve git history, I think it'll be better if you squash and rebase your git commits.
I know is a personal opinion, but here is a good article on why is that: https://blog.carbonfive.com/always-squash-and-rebase-your-git-commits/

As you might know, English is not my first language and I'm sorry if I'm not good enough to just leave a suggestion on this personal comment.

@pellared
Copy link
Member Author

pellared commented Jul 4, 2020

Thanks a lot for review and feedback.

Usually I squash the commits and merge and sometimes just rebase and merge. Depends on the PR 😉
You may be interested in https://www.conventionalcommits.org/en/v1.0.0/ as well as https://chris.beams.io/posts/git-commit/ .

@pellared pellared merged commit c3db17c into master Jul 4, 2020
@pellared pellared deleted the coverage branch July 4, 2020 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants